Skip to content

Rack 3.x compatibility #16158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Rack 3.x compatibility #16158

wants to merge 2 commits into from

Conversation

@voxik
Copy link

@voxik voxik commented Aug 11, 2025 •
edited by qodo-merge-pro bot
Loading

User description

🔗 Related Issues

💥 What does this PR do?

Selenium locks Rack to version 2.x, whereas the Rack 3.x is the future. This PR attempts to address the compatibility with Rack 3+

🔧 Implementation Notes

This hopefully keeps compatibility with Rack 2.x. And keeps the Rack 2.x to be locked in Gemfile.lock.

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)

Not sure what category this belongs to 🤔

PR Type

Enhancement

Description

  • Update Rack compatibility for version 3.x support

  • Replace deprecated Rack::File with Rack::Files

  • Add conditional Rackup::Handler usage for Rack 3+

  • Maintain backward compatibility with Rack 2.x

Diagram Walkthrough

flowchart LR
  A["Rack 2.x"] --> B["Compatibility Layer"]
  C["Rack 3.x"] --> B
  B --> D["Rack::Files"]
  B --> E["Rackup::Handler"]
  D --> F["Test Server"]
  E --> F
Loading

File Walkthrough
Relevant files
Enhancement
rack_server.rb
Update Rack server for 3.x compatibility                                 

rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb

  • Add conditional rackup require with error handling
  • Replace Rack::File with Rack::Files for compatibility
  • Use Rackup::Handler when available, fallback to Rack::Handler
 +6/-2     

@CLAassistant
Copy link

CLAassistant commented Aug 11, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@selenium-ci selenium-ci added the C-rb Ruby Bindings label Aug 11, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 11, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

  • None

Non-compliant requirements:

  • Investigate and resolve connection failures for multiple ChromeDriver instances
  • Ensure subsequent ChromeDriver instantiations do not fail
  • Provide reproducible steps or fixes for stabilizing connections

Requires further human verification:

  • N/A

1234 - Partially compliant

Compliant requirements:

  • None

Non-compliant requirements:

  • Ensure click() triggers JavaScript in link href in Firefox

Requires further human verification:

  • N/A
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Compatibility Path

Using Rackup::Handler when available changes handler resolution; verify this works across Rack 2.x and 3.x and with all listed handlers (thin/mongrel/webrick) and that constants match in both namespaces.

 
def handler
  # can't use Platform here since it's being run as a file on Windows + IE.
  handlers = if RUBY_PLATFORM.match?(/mswin|msys|mingw32/)
               %w[mongrel webrick]
             else
               %w[thin mongrel webrick]
             end

  handler = handlers.find { |h| load_handler h }
  constant = handler == 'webrick' ? 'WEBrick' : handler.capitalize
  defined?(Rackup) ? Rackup::Handler.const_get(constant) : Rack::Handler.const_get(constant)
end
Optional Require

Silent rescue on requiring 'rackup' may hide load issues; consider logging or feature-detecting more explicitly to avoid unexpected fallback behavior.

 
begin
  require 'rackup'
rescue LoadError
end
Middleware Change

Replacing Rack::File with Rack::Files can alter header behavior and directory listing semantics; validate static file serving works identically for tests across Rack versions.

 
class TestApp
  BASIC_AUTH_CREDENTIALS = %w[test test].freeze

  def initialize(file_root)
    @static = Rack::Files.new(file_root)
  end

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 11, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard against missing handler

If no handler can be loaded, handler will be nil and nil.capitalize will raise.
Add a guard to raise a clear error when no supported handler is available to
prevent a misleading NoMethodError.

rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb [81-83]

 handler = handlers.find { |h| load_handler h }
+raise "No supported Rack handler found (tried: #{handlers.join(', ')})" unless handler
 constant = handler == 'webrick' ? 'WEBrick' : handler.capitalize
-defined?(Rackup) ? Rackup::Handler.const_get(constant) : Rack::Handler.const_get(constant)
+(defined?(Rackup) ? Rackup::Handler : Rack::Handler).const_get(constant)
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a potential NoMethodError if no handler is found and proposes adding a guard clause, which significantly improves error handling and robustness.

Medium
General
Ensure absolute static root

Rack::Files expects an absolute root path; passing a relative path can cause
404s depending on CWD. Normalize file_root to an absolute path to ensure
consistent static file serving.

rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb [107]

-@static = Rack::Files.new(file_root)
+@static = Rack::Files.new(File.expand_path(file_root))
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that Rack::Files works best with an absolute path and proposes using File.expand_path to prevent potential issues, which improves the code's robustness.

Medium
Avoid silent LoadError swallow

Swallowing the LoadError silently can mask misconfiguration and lead to
confusing behavior. Log a concise debug message or set an explicit flag to
indicate Rackup is unavailable so downstream code paths are clear and test
failures are easier to diagnose.

rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb [21-24]

 begin
   require 'rackup'
+  RACKUP_AVAILABLE = true
 rescue LoadError
+  RACKUP_AVAILABLE = false
+  warn('[RackServer] rackup gem not available; falling back to Rack::Handler') if ENV['SELENIUM_DEBUG']
 end
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out that swallowing LoadError silently can hinder debugging, but the proposed RACKUP_AVAILABLE flag is redundant as the existing code already uses defined?(Rackup) for the same purpose.

Low
Learned
best practice
Guard constant access with checks

Guard the Rackup::Handler access with a safer check to ensure the constant is
available and fallback gracefully. Using defined?(Rackup::Handler) avoids
potential NameError when rackup loads but Handler is missing or renamed.

rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb [83]

-defined?(Rackup) ? Rackup::Handler.const_get(constant) : Rack::Handler.const_get(constant)
+if defined?(Rackup::Handler)
+  Rackup::Handler.const_get(constant)
+else
+  Rack::Handler.const_get(constant)
+end
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add null checks and validation for parameters and variables before using them to prevent runtime errors.

Low
  • Update

@voxik
Use Rack::Files for Rack 3+ compatibility
027eb7a 
Rack has renamed `Rack::File` to `Rack::Files` starting with Rack 2.1.0
[[1]]. Rack 3+ drops the old `Rack::File` class and therefore there are
bunch of errors such as:

~~~
  334) Selenium::WebDriver::VirtualAuthenticator#user_verified= can not obtain credential requiring verification when set to false
       Failure/Error: @static = Rack::File.new(file_root)

       NameError:
         uninitialized constant Rack::File
       # ./spec/integration/selenium/webdriver/spec_support/rack_server.rb:103:in 'Selenium::WebDriver::SpecSupport::RackServer::TestApp#initialize'
       # ./spec/integration/selenium/webdriver/spec_support/rack_server.rb:30:in 'Class#new'
       # ./spec/integration/selenium/webdriver/spec_support/rack_server.rb:30:in 'Selenium::WebDriver::SpecSupport::RackServer#initialize'
       # ./spec/integration/selenium/webdriver/spec_support/test_environment.rb:87:in 'Class#new'
       # ./spec/integration/selenium/webdriver/spec_support/test_environment.rb:87:in 'Selenium::WebDriver::SpecSupport::TestEnvironment#app_server'
       # ./spec/integration/selenium/webdriver/spec_support/test_environment.rb:162:in 'Selenium::WebDriver::SpecSupport::TestEnvironment#url_for'
       # ./spec/integration/selenium/webdriver/spec_support/helpers.rb:41:in 'Selenium::WebDriver::SpecSupport::Helpers#url_for'
       # ./spec/integration/selenium/webdriver/virtual_authenticator_spec.rb:55:in 'block (2 levels) in <module:WebDriver>'
~~~

[1]: rack/rack@626272b
@voxik
Use Rackup::Handler when available
f470b1c 
The `Rack::Handler` was extracted from Rack 3+ into separate `rackup`
package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

C-rb Ruby Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@voxik @CLAassistant @selenium-ci